Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Context Window Exceeded fix #4977

Merged
merged 16 commits into from
Nov 14, 2024
Merged

Context Window Exceeded fix #4977

merged 16 commits into from
Nov 14, 2024

Conversation

enyst
Copy link
Collaborator

@enyst enyst commented Nov 13, 2024

End-user friendly description of the problem this fixes or functionality that this introduces

  • Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below
    Truncate agent history when context window is exceeded

Give a summary of what the PR does, explaining any non-trivial design decisions

This PR proposes a simple fix for the ContextWindowExceededError:

  • when raised, truncate the context roughly in half
  • save state and reload the truncated history when session is restored

Please note that this is not well tested, but it is ready for code review.


Link of any specific issues this addresses
Fix #4951
Fix #4894


To run this PR locally, use the following command:

docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:2fcb088-nikolaik   --name openhands-app-2fcb088   docker.all-hands.dev/all-hands-ai/openhands:2fcb088

@enyst enyst requested a review from rbren November 13, 2024 19:13
@enyst enyst force-pushed the enyst/context-limit-fix branch from 00597d6 to d85debd Compare November 13, 2024 19:16
Copy link
Contributor

@neubig neubig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, thank you for doing it!

It'd be nice if we could do an eval before merging just to make sure that things are working, I added the label now.

Copy link
Contributor

Hi! I started running the evaluation on your PR. You will receive a comment with the results shortly.

@enyst
Copy link
Collaborator Author

enyst commented Nov 13, 2024

Unfortunately it looks like the Deepseek eval workflow doesn't work on main. 🤔 I seem to recall that we had merged it with high hopes, but function calling didn't actually work with Deepseek, so Deepseek suddenly wasn't so cool anymore 😅 and this workflow was not really used yet afaik.

But I can submit it to a number of good tests tomorrow!

@neubig
Copy link
Contributor

neubig commented Nov 13, 2024

I think we don't need to test then, this should be fine!

@enyst enyst force-pushed the enyst/context-limit-fix branch from b6ec632 to 0b7c08d Compare November 14, 2024 01:34
@enyst enyst force-pushed the enyst/context-limit-fix branch from 0b7c08d to 2fcb088 Compare November 14, 2024 02:22
@enyst enyst enabled auto-merge (squash) November 14, 2024 02:23
@enyst enyst merged commit 8dee334 into main Nov 14, 2024
12 checks passed
@enyst enyst deleted the enyst/context-limit-fix branch November 14, 2024 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is there a way to prevent openhands from crashing at 200k context? [Bug]: litellm.BadRequestError
2 participants